feat: Allow concurrent payload visiting#2230
feat: Allow concurrent payload visiting#2230brucearctor wants to merge 1 commit intotemporalio:masterfrom
Conversation
|
Hi @brucearctor. Thanks for taking time to create a PR for this issue and for your contribution. The intent of temporalio/features#772 was to update https://github.com/temporalio/api-go/blob/master/proxy/interceptor.go#L101C6-L101C19 to enable concurrent visiting so that it is broadly usable. This would allow concurrency regardless of the visitor callback implementation. gRPC interceptors are one type of these callback implementations. The concurrent visiting was enabled upstream in temporalio/api-go#244 and integrated into the current repository in #2294. The gRPC interceptors themselves are not specifically aware of the concurrency enablement, but can be made concurrent using the payload visitor concurrency options. Since the interceptors do not need concurrent visiting explicitly enabled within their implementations and can leverage the generalized concurrent visiting options, we will not accepting this PR in its current form. If you feel there are additional improvements that could be made in this space, please do not hesitate to open an issue for discussion. |
|
Makes sense. And I approx the response! |
Closes #2223
This PR introduces a
Concurrencyconfiguration option insidePayloadCodecGRPCClientInterceptorOptionsandNewFailureGRPCClientInterceptorOptions.When
Concurrency > 1, the proxy visitor collects payload pointers sequentially but decodes/encodes payloads concurrently usingerrgroup, reducing latency drastically when the codec includes heavy IO (e.g., KMS client-side encryption calls).It avoids deep copies by directly modifying the byte vectors in place on the tree.
Fallback execution remains entirely sequential if
Concurrency=1or0.A unit test asserting the parallel time reduction is included.